Skip to content

Conversation

@tahminator
Copy link
Owner

@tahminator tahminator commented Jan 16, 2026

641

Description of changes

Checklist before review

  • I have done a thorough self-review of the PR
  • Copilot has reviewed my latest changes, and all comments have been fixed and/or closed.
  • If I have made database changes, I have made sure I followed all the db repo rules listed in the wiki here. (check if no db changes)
  • All tests have passed
  • I have successfully deployed this PR to staging
  • I have done manual QA in both dev (and staging if possible) and attached screenshots below.

Screenshots

Dev

Staging

@github-actions
Copy link
Contributor

Available PR Commands

  • /ai - Triggers all AI review commands at once
  • /review - AI review of the PR changes
  • /describe - AI-powered description of the PR
  • /improve - AI-powered suggestions
  • /deploy - Deploy to staging

See: https://github.com/tahminator/codebloom/wiki/CI-Commands

@github-actions
Copy link
Contributor

Title

641: wip


PR Type

Enhancement, Other


Description

  • Replace bash CI script with Bun TypeScript

  • Add Node, pnpm, Bun setup in workflow

  • Introduce scripts package with lockfile

  • Add tsconfig for CI scripts


Diagram Walkthrough

flowchart LR
  BashScript["Bash script (.sh)"] -- replaced by --> BunTS["Bun TypeScript script (.ts)"]
  Workflow["GitHub Action composite"] -- setup Node/pnpm/Bun --> Env["Script runtime env"]
  Env -- installs --> Deps["scripts package deps"]
  BunTS -- runs --> Maven["Maven fmt/lint/verify"]
Loading

File Walkthrough

Relevant files
Enhancement
run-backend-compile-tests.ts
Add Bun TS script to run Maven checks                                       

.github/scripts/run-backend-compile-tests.ts

  • Add Bun TypeScript script runner
  • Execute Maven spotless, checkstyle, and verify
  • Provide success/failure exit handling
+21/-0   
Miscellaneous
run-backend-compile-tests.sh
Remove legacy Bash CI script                                                         

.github/scripts/run-backend-compile-tests.sh

  • Remove Bash script invoking Maven tasks
+0/-13   
Configuration changes
action.yml
Update composite action to use Bun and pnpm                           

.github/composite/test/backend-pre-test/action.yml

  • Setup pnpm and Node cache
  • Install Bun runtime
  • Install script dependencies with pnpm
  • Switch run step to Bun TS script
+17/-1   
tsconfig.json
Add TypeScript configuration for CI scripts                           

.github/scripts/tsconfig.json

  • Add TypeScript config for scripts
  • Enable strict, JS checking, and path aliases
+23/-0   
Dependencies
package.json
Introduce package.json for CI scripts                                       

.github/scripts/package.json

  • Add scripts package metadata
  • Declare bun and @types/bun dependencies
  • Specify pnpm as packageManager
+16/-0   
pnpm-lock.yaml
Add pnpm lockfile for scripts deps                                             

.github/scripts/pnpm-lock.yaml

  • Add lockfile for scripts dependencies
  • Pin Bun and types packages
+155/-0 

@tahminator tahminator force-pushed the 641 branch 2 times, most recently from 67c5fe1 to a6ba20b Compare January 16, 2026 01:04
@github-actions
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Execution Shell Mismatch

The step invokes bun .github/scripts/run-backend-compile-tests.ts but the step's shell is set to bash. While GitHub defaults to using the runner shell regardless, explicitly specifying bash can be misleading and unnecessary for a Bun command. Consider removing shell or setting it appropriately to avoid confusion.

- name: Run script
  shell: bash
  run: bun .github/scripts/run-backend-compile-tests.ts
Unnecessary Dependency

Adding bun as a runtime dependency may be redundant since the workflow installs Bun via oven-sh/setup-bun. Keeping it in dependencies bloats lockfile and can cause version drift with the action. Prefer relying on the action and removing the bun dependency, keeping only types in devDependencies if needed.

"dependencies": {
    "bun": "^1.3.6"
},
"devDependencies": {
    "@types/bun": "^1.3.6"
}
Caching/Install Scope

pnpm --dir .github/scripts install --frozen-lockfile installs within the .github/scripts directory, but the workflow sets Node cache at repo root. Ensure cache keys and working dir align or set actions/setup-node cache-dependency-path to .github/scripts/pnpm-lock.yaml to get effective caching.

- uses: actions/setup-node@v4
  with:
    node-version: 20
    cache: "pnpm"

- uses: oven-sh/setup-bun@v2
  with:
    bun-version: latest

- name: Install deps
  run: pnpm --dir .github/scripts install --frozen-lockfile

await $`./mvnw checkstyle:check`;

// compile
await $`./mvnw -B verify -Dmaven.test.skip=true --no-transfer-progress`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Use 'with' on '$' to fail early and stream output; also set a working directory to the repo root to avoid path issues. This ensures the Maven command inherits stdio and fails on non-zero exit reliably in Bun contexts. [possible issue, importance: 6]

Suggested change
await $`./mvnw -B verify -Dmaven.test.skip=true --no-transfer-progress`;
await $.cwd(process.cwd()).with({ stdin: 'inherit', stdout: 'inherit', stderr: 'inherit' })`./mvnw -B verify -Dmaven.test.skip=true --no-transfer-progress`;

Comment on lines 14 to 21
main()
.then(() => {
process.exit(0);
})
.catch((e) => {
console.error(e);
process.exit(1);
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Avoid explicit process.exit calls; they can terminate before streams flush and mask unhandled rejections. Let the promise chain determine exit status by rethrowing or returning non-zero via shell. [general, importance: 7]

Suggested change
main()
.then(() => {
process.exit(0);
})
.catch((e) => {
console.error(e);
process.exit(1);
});
main().catch((e) => {
console.error(e);
// Non-zero exit by setting process.exitCode; allows stdout/stderr to flush.
process.exitCode = 1;
});

Comment on lines 36 to 39
- name: Install deps
shell: bash
run: pnpm --dir .github/scripts install --frozen-lockfile

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Pin the working directory using 'working-directory' instead of relying on '--dir' to avoid path resolution differences across runners. This reduces risk of installing in the wrong location. [general, importance: 6]

Suggested change
- name: Install deps
shell: bash
run: pnpm --dir .github/scripts install --frozen-lockfile
- name: Install deps
shell: bash
working-directory: .github/scripts
run: pnpm install --frozen-lockfile

@codecov
Copy link

codecov bot commented Jan 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@tahminator tahminator force-pushed the 641 branch 2 times, most recently from a8a8e6f to 118e014 Compare January 16, 2026 02:55
@tahminator tahminator force-pushed the 641 branch 2 times, most recently from 83c86aa to 933567d Compare January 16, 2026 03:34
@tahminator
Copy link
Owner Author

/ai

@tahminator
Copy link
Owner Author

/review

@tahminator
Copy link
Owner Author

/describe

@tahminator
Copy link
Owner Author

/improve

@github-actions
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Error handling

If postgres doesn't become ready or migration fails, the script logs and calls end() but does not exit with non-zero status, which may let CI proceed despite failure. Consider throwing after end() or process.exit(1) to fail fast.

  if (!ready) {
    console.error("postgres failed to start in time.");
    end();
  }

  process.env.DATABASE_HOST = "localhost";
  process.env.DATABASE_PORT = "5440";
  process.env.DATABASE_NAME = "codebloom";
  process.env.DATABASE_USER = "postgres";
  process.env.DATABASE_PASSWORD = "postgres";

  console.log("postres started, running migrations...");

  await $`./mvnw flyway:migrate -Dflyway.locations=filesystem:./db`;

  console.log("postgres ready");
} catch (e) {
  console.error(e);
  end();
}
Process cleanup

start() calls end() on failure but does not await it, and does not set non-zero exit; also be.kill() may not terminate child tree (mvn + java). Consider awaiting end(), ensuring non-zero exit, and using proper signal/kill group if needed.

  if (!ready) {
    console.error("Backend failed to start in time.");
    end();
  }

  console.log("backend ready");
} catch (e) {
  console.error(e);
  end();
}
Shell step portability

Uses a compound shell command with cd and script invocation in one template literal; if path contains spaces or if run from a different CWD, this may be brittle. Prefer --cwd flags or separate steps to ensure reliable execution.

await $`corepack enable pnpm`;
await $`cd email && pnpm i --frozen-lockfile && ./email.sh && cd ..`;

@github-actions
Copy link
Contributor

Title

641: wip


PR Type

Enhancement


Description

  • Replace bash scripts with Bun TypeScript

  • Add ANSI color utilities for logs

  • Update CI to setup and use Bun

  • Improve AI review with Notion context


Diagram Walkthrough

flowchart LR
  Bash["Legacy bash scripts"] -- replaced by --> BunTS["Bun TypeScript scripts"]
  BunTS -- used in --> CIBackend["CI: Backend tests"]
  BunTS -- used in --> CIFrontend["CI: Frontend tests"]
  Colors["ANSI color helpers"] -- improve logs --> BunTS
  Workflows["GitHub Actions workflows"] -- setup bun/cache/run --> CIBackend
  Workflows -- setup bun/cache/run --> CIFrontend
  AIReview["AI review workflow"] -- add Notion context & tuning --> Workflows
Loading

File Walkthrough

Relevant files
Enhancement
6 files
colors.ts
Add ANSI color formatting helpers                                               
+103/-0 
run-backend-instance.ts
Implement backend runner with readiness checks                     
+73/-0   
run-local-db.ts
Implement Postgres lifecycle and migration runner               
+84/-0   
run-backend-compile-tests.ts
Port backend compile/lint to Bun script                                   
+21/-0   
run-backend-tests.ts
Port backend tests pipeline to Bun                                             
+29/-0   
run-frontend-tests.ts
Port frontend e2e tests pipeline to Bun                                   
+27/-0   
Miscellaneous
5 files
local-db.sh
Remove bash-based local DB management                                       
+0/-52   
run-backend-compile-tests.sh
Remove bash backend compile script                                             
+0/-13   
run-backend-instance.sh
Remove bash backend instance runner                                           
+0/-38   
run-backend-tests.sh
Remove bash backend tests script                                                 
+0/-26   
run-frontend-tests.sh
Remove bash frontend tests script                                               
+0/-15   
Configuration changes
4 files
action.yml
Setup Bun and run TS compile script                                           
+18/-1   
tsconfig.json
Add TypeScript config for Bun scripts                                       
+29/-0   
ai-review.yml
Add Notion context and tune PR reviewer                                   
+19/-1   
ci-cd.yml
Use Bun in CI for backend/frontend scripts                             
+36/-7   
Dependencies
1 files
package.json
Add scripts package with Bun deps                                               
+15/-0   

641: wip fix coloring

641: wip fix coloring pt2

641: update ai review

641: add load-secrets

641: add load-secrets pt2

641: add load-secrets pt3

641: add load-secrets pt4

641: add load-secrets pt5 fix writing logic

641: add load-secrets pt5 fix writing logic wdekfjnewkgfn
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants